Add optional Git hook to run lint before pushing #175
Conversation
Say you've completed work on your branch and now you want to create a pull request, but unbeknownst to you, some code doesn't pass the lint step. It's nice to address those lint violations now, otherwise you will end up waiting until CI runs and puts an X on your PR. Some of us on the Shared Libraries team have been testing using a Git hook to run the lint step in the `core` repo, and it has been working well so far, so this commit brings the same idea over. The Git hook runs before each push, allowing commit creation to remain speedy, and it is a completely optional part of setting up your development environment. You can install it by running `yarn sync-git-hooks`.
|
New dependency changes detected. Learn more about Socket for GitHub ↗︎ 🚨 Potential security issues found in this pull request. To accept the risk, merge this PR and you will not be notified again. Bot CommandsTo ignore an alert, reply with a comment starting with
📜 Install scriptsInstall scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts. Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.
Pull request alert summary
📊 Modified Dependency Overview:
|
mcmire
force-pushed
the
add-simple-git-hooks
branch
from
f5fb827
to
1721174
Compare
| "test": "jest && jest-it-up", | ||
| "test:watch": "jest --watch" | ||
| }, | ||
| "simple-git-hooks": { | ||
| "pre-push": "yarn lint" |
There was a problem hiding this comment.
I would prefer this to be pre-commit. If you separate the commands a little bit, it could run eslint --fix and prettier --write as well, using lint-staged. See snaps-monorepo for example.
There was a problem hiding this comment.
If changing to pre-commit, maybe we can add a note in README.md on the existence of git commit -n to make onboarding smoother?
There was a problem hiding this comment.
I definitely agree with using some kind of mechanism to lint fewer files. That would go a long way in improving the developer experience, I believe, and would possibly remove the concerns I have about using a pre-commit hook. lint-staged requires that you use a .prettierignore file which I'd like to avoid. I figured out some other way of doing this for another project, however, so I'll see if I can find that.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
I am going to close this as it's very stale now, and this isn't a high priority. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Say you've completed work on your branch and now you want to create a pull request, but unbeknownst to you, some code doesn't pass the lint step. It's nice to address those lint violations now, otherwise you will end up waiting until CI runs and puts an X on your PR.
Some of us on the Shared Libraries team have been testing using a Git hook to run the lint step in the
corerepo, and it has been working well so far, so this commit brings the same idea over. The Git hook runs before each push, allowing commit creation to remain speedy, and it is a completely optional part of setting up your development environment. You can install it by runningyarn sync-git-hooks.